Conversation
github-actions
Bot
added
the
area:workflows
naivezip
changed the title
…ild inheritance (openpi-dev#483) pi-intercom is excluded from child sessions via blockedPackageSources to prevent concurrent process.env session cross-wiring (openpi-dev#128). However, when pi-intercom was active in the parent session, inheritedChildToolAllowlist projected its tools into child sessions, causing subagent_spawn and workflow child sessions to fail preflight checks. Rather than hardcoding foreign tool names into OpenPI's CHILD_EXCLUDED_TOOL_NAMES (which is strictly reserved for OpenPI-owned parent-only tools and enforced by the fail-closed drift guard), dynamically drop tools from blocked packages during child inheritance: - Inspect tool.sourceInfo in inheritedChildToolAllowlist against blocked child package matchers - Pass pi.getAllTools() and cwd context from subagents/index.ts and workflows/index.ts - Preserve CHILD_EXCLUDED_TOOL_NAMES and bidirectional drift guards strictly for OpenPI tools - Add regression coverage verifying dynamic exclusion of tools from blocked packages while preserving ordinary tools and passing child preflight
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed at exact head 88531a1.
Standards
[P1] Unverifiable tool provenance currently fails open. The new local descriptor makes sourceInfo and availableTools optional, and isBlockedChildTool() returns false when package identity inspection throws. A blocked-package tool with missing or unreadable provenance is therefore treated as child-safe. Pi 0.85.1 provides getAllTools() with sourceInfo; please use that authoritative type non-optionally and deny/throw when non-builtin provenance cannot be verified.
[P2] Blocked package identity is implemented twice with different conditions: blockedPackageSources() filters canonical resources, while isBlockedChildTool() independently hardcodes pi-intercom matching and ignores origin. Extract one shared blocked-package identity policy for both projections.
Spec
[P2] The regression covers a helper and synthetic preflight only. It does not execute Direct spawn, typed spawn, or Workflow callers with a real local/npm/git pi-intercom fixture, so omission or caller-plumbing drift would go undetected. Because this changes the child model-visible authority boundary, please add caller-level Direct and Workflow tests.
[P3] The array shorthand and unused agentDir generality are outside the required production path.
|
A follow-up implementation is available in #528, rebased onto current The correction uses required Pi tool provenance, shares the loaded-resource filtering policy, removes the catch-and-guess fallback, and resolves nested single-file local sources through Pi's scoped installed-path lookup. It also adds real-parent-metadata Direct/typed Direct and Workflow caller regressions. The extended package regression fails at this PR's current head ( #528 includes the original change plus the corrections, so maintainers can review the combined result there without needing to merge both PRs independently. Please preserve both authors' attribution if squashing. |
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
审查提交：88531a124cd6f8fa129368f842f66796a9468ad3。
需要修改后再合并。
具体问题
P2 — 来源无法验证时仍保留请求工具，阻止子 Session 的正常启动 · extensions/shared/child-session.ts:585
sourceInfo/availableTools optional and inspection catch returns false for opaque local paths. This retains excluded names; subsequent bindChildSessionExtensions missing-tool check rejects startup. Use authoritative nonoptional Pi ToolInfo and explicit provenance failure; current tests only mock descriptors.
验证范围
Full diff and Pi 0.85.1 getAllTools source metadata/preflight inspected; 20 helper tests pass. Existing review concerns still unaddressed at same head.
限制与后续
Do not claim authority escalation: child resource loader and preflight remain fail-closed. No actual local/npm/git package integration reproduction performed.
|
Closing as superseded: this change landed on |
3 participants
Problem
Fixes #483. Also addresses the preflight root cause reported in #493.
When
@tt-a1i/openpiis co-installed with third-party extensions such aspi-intercom,subagent_spawn(including standard subagents without explicitagent_type) and Workflow child sessions fail during startup preflight before the first model turn:Root cause:
blockedPackageSources()stripspi-intercomfrom child session extensions/packages to avoid cross-wiring session identity (per #128). However, during child tool allowlist resolution,inheritedChildToolAllowlist(pi.getActiveTools(), roleTools)blindly inherited all active parent tools unless they were explicitly listed in OpenPI's internalCHILD_EXCLUDED_TOOL_NAMES. Because foreign tools likeintercomdo not belong in OpenPI's internal tool surface (and adding them there violates the bidirectional fail-closed drift guard), tools from blocked packages leaked into the child requested tool list where they could never be satisfied.Value
pi-intercomare present in the parent environment.#128without breaking the bidirectional drift guard or requiring ongoing manual maintenance of third-party tool names.Approach
child-session.ts:CHILD_EXCLUDED_TOOL_NAMESstrictly scoped to OpenPI-owned parent tools, preserving the bidirectional partition invariant (CHILD_SAFE_PACKAGE_TOOL_NAMES+CHILD_EXCLUDED_TOOL_NAMES==OPENPI_TOOL_SURFACE_NAMES).ChildToolInheritanceOptions({ availableTools?: readonly PiToolDescriptor[]; cwd?: string }) andisBlockedChildTool(tool, options)to inspect tool provenance (tool.sourceInfo) against blocked child package matchers (createPiIntercomPackageMatcher).inheritedChildToolAllowlist, if available tool descriptors are provided, dynamically drop any tools originating from blocked child packages.extensions/subagents/index.ts, pass{ availableTools: pi.getAllTools?.(), cwd: ctx.cwd }intoinheritedChildToolAllowlist.extensions/workflows/index.ts, pass{ availableTools: pi.getAllTools?.(), cwd: ctx.cwd }intoinheritedChildToolAllowlist.tests/extensions/shared/child-session.test.ts.intercomfrompi-intercom) are dynamically excluded during inheritance and allow child sessions to pass preflight.tests/extensions/shared/tool-surface.test.tsandtests/extensions/subagents/agent-types.test.ts.Validation
node --test --experimental-strip-types tests/extensions/shared/child-session.test.ts: 20/20 passednode --test --experimental-strip-types tests/extensions/shared/tool-surface.test.ts: 9/9 passednode --test --experimental-strip-types tests/extensions/subagents/agent-types.test.ts: 22/22 passednode --test --experimental-strip-types tests/extensions/subagents/index.test.ts: 16/16 passednode --test --experimental-strip-types tests/extensions/subagents/pi-backend.test.ts: 5/5 passednode --test --experimental-strip-types tests/extensions/workflows/runner.test.ts: 26/26 passednode scripts/check-config-contract.mjs: 15/15 fields passednode scripts/check-discipline-ledger.mjs: 12/12 rows passednpx biome format&npx biome lint --error-on-warnings: Clean, 0 errorsgit diff upstream/main..HEAD: Exactly 4 files changed (+183, -1)Impact
pi-intercomis co-installed in the parent.